Skip to content

Route owned PR events to babysitter sessions#92

Merged
miyaontherelay merged 2 commits into
mainfrom
codex/issue-87-babysitter-events
Jul 17, 2026
Merged

Route owned PR events to babysitter sessions#92
miyaontherelay merged 2 commits into
mainfrom
codex/issue-87-babysitter-events

Conversation

@miyaontherelay

Copy link
Copy Markdown
Contributor

Summary

  • route canonical GitHub review, review-comment, issue-comment, failed-check, conflict, and base-divergence activity only to the babysitter that owns the exact normalized repo and PR
  • coalesce metadata-only wakes with confirmed delivery, retry, critical-section fencing, and pull/poll correctness backstops
  • persist exact babysitter ownership, pending wake categories, and critical fences across process restarts

Stack

Depends on #89. This PR is intentionally stacked on branch codex/issue-82-repo-agent-names and preserves its repo-qualified agent naming plus composite issue identity. It must not target main until #89 merges; after that, it requires rebase/integration and a fresh frozen shadow delta audit before changing the base.

Fixes #87

Security and lifecycle

  • canonical flat review, comment, and check records require consistent path object ID plus validated repo and PR identity; mismatched or ambiguous records fail closed
  • provider-authored titles, bodies, comments, URLs, check names, and control text are excluded from wake prompts
  • pending and green checks do not wake; failed, cancelled, and timed-out checks do
  • an explicit confirmed Factory ACK causally installs and durably records the no-submit fence before destructive babysitter work
  • terminal close/completion cancels queued work and clears durable session state

Validation

  • focused babysitter, prompt, durable-state, and CLI tests: 41 passed, 309 skipped
  • full test suite: 38 files, 759 passed
  • npm run build
  • npm pack --dry-run --json: 262 files with dist main/types and CLI bin
  • git diff --check
  • independent frozen shadow audit: signed off exact binary diff SHA-256 c1b5fbe992859fd43ba1a765abd0a956cea8ffb7e4880142d04b9fdd2c40fa05

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@miyaontherelay, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ddacefc1-c06d-41a8-8c73-ee2a3169954f

📥 Commits

Reviewing files that changed from the base of the PR and between e41931e and 7bb575d.

📒 Files selected for processing (5)
  • src/orchestrator/factory.test.ts
  • src/orchestrator/factory.ts
  • src/state/file-state-store.test.ts
  • src/state/file-state-store.ts
  • src/triage/triage.test.ts
📝 Walkthrough

Walkthrough

Changes

PR babysitter wake flow

Layer / File(s) Summary
Session state contracts and persistence
src/ports/*, src/triage/schema.ts, src/state/*
Adds babysitter ownership and pending-wake shapes, in-memory and file-backed persistence, validation, migration, and persistence tests.
Ownership and event routing
src/orchestrator/factory.ts
Tracks PR ownership, restores sessions, parses GitHub events and status, routes matching events, and prevents ownership conflicts during spawning.
Wake delivery and critical fencing
src/orchestrator/factory.ts
Coalesces and retries wakes, persists pending kinds, defers delivery during destructive work, processes acknowledgments, and cleans up wake state.
Prompt protocol and integration coverage
src/dispatch/*, src/orchestrator/factory.test.ts
Updates babysitter instructions and tests routing isolation, metadata fencing, critical ordering, retries, restart recovery, and cancellation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHub
  participant FactoryLoop
  participant BabysitterSession
  GitHub->>FactoryLoop: owned PR review or check event
  FactoryLoop->>FactoryLoop: coalesce wake kinds and check critical fence
  FactoryLoop->>BabysitterSession: inject metadata-only wake
  BabysitterSession-->>FactoryLoop: critical begin/end signal
  FactoryLoop->>BabysitterSession: submit deferred wake
Loading

Possibly related PRs

Suggested reviewers: khaliqgant

Poem

I’m a rabbit with a PR to mind,
Wake signals hop in a careful line.
Fences guard each risky chore,
Pending carrots wait at the door.
Restarted burrows remember the way—
One tidy thump, then back to play.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes routing owned PR events into babysitter sessions.
Description check ✅ Passed The description directly matches the event routing, coalescing, fencing, and persistence changes.
Linked Issues check ✅ Passed The changes implement the owned-PR event delivery, coalescing, critical-section fencing, and durable state requested by #87.
Out of Scope Changes check ✅ Passed The diff stays focused on babysitter event routing, state persistence, schema updates, and tests with no clear unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-87-babysitter-events

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Base automatically changed from codex/issue-82-repo-agent-names to main July 17, 2026 07:35
@miyaontherelay
miyaontherelay force-pushed the codex/issue-87-babysitter-events branch from 61fa3e3 to e41931e Compare July 17, 2026 07:58

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/orchestrator/factory.ts (1)

7676-7679: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reject PR numbers that disagree with the canonical path.

payload.number silently overrides fallbackNumber. Thus content for /pulls/420/... can claim PR 421, producing ownership for 421 while persisting the path for 420.

Proposed fix
 const parsePullSnapshot = (content: unknown, fallbackNumber: number): PullSnapshot | undefined => {
   const payload = wrappedPayload(content)
-  const number = typeof payload.number === 'number' ? payload.number : fallbackNumber
+  const explicitNumber = payload.number === undefined
+    ? undefined
+    : positiveIntegerLike(payload.number)
+  if (payload.number !== undefined && explicitNumber !== fallbackNumber) return undefined
+  const number = explicitNumber ?? fallbackNumber
   if (!Number.isInteger(number) || number <= 0) return undefined
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/orchestrator/factory.ts` around lines 7676 - 7679, Update
parsePullSnapshot so fallbackNumber from the canonical path is authoritative,
rather than allowing payload.number to override it. Validate the payload number
when present and return undefined if it is not an integer or disagrees with
fallbackNumber; otherwise continue using fallbackNumber for the resulting
PullSnapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/orchestrator/factory.ts`:
- Around line 3149-3152: In the babysitter critical-section exit flow around
`#finishBabysitterCriticalSection` and deferred wake processing, persist the
cleared critical fence immediately after consuming the end signal and before
submitting any deferred PTY input or wakes. Preserve the existing deferred
submission behavior, but ensure `#persistBabysitterCriticalFence` runs before that
processing so a crash cannot restore a permanent fence.
- Around line 4137-4149: The delivery-failure handling near the wake-delivery
path must check whether the wake was cancelled before restoring kinds or
persisting its session. Update the logic around `#cancelBabysitterWake` and the
failure branch to skip session recreation and related recovery for cancelled
state, preserving cleanup for active wakes.
- Around line 4495-4498: The readiness handling around the babysitter-enabled
guard must reject stale signals after PR ownership is cancelled. Before
resolving the agent, require that the babysitter still has active ownership and
that `#readBabysatPrSnapshot`() returns an authoritative snapshot; otherwise
cancel or terminate the babysitter and return. Preserve the existing non-OPEN
snapshot cancellation behavior.
- Around line 4505-4517: Update the ownership establishment flow around
`#inFlightIssueForPrSnapshot` and the babysitter reservation at the referenced
ownership block so a first match cannot claim ownership from title/body
references alone. Require either authoritative published-PR identity or a strong
head-branch association before creating the initial babysitter ownership; reject
weak same-repository references such as only “Fixes AR-420” when no existing
owner is present, while preserving established-owner mismatch handling.
- Around line 4331-4343: Update the babysitter wake transition around
`#recordPendingBabysitterWake` and `#flushBabysitterWake` so failures are handled by
recovery logic covering the entire flush sequence. On rejection, restore
deliveringKinds into kinds, schedule a retry through `#scheduleBabysitterWake`,
and ensure the promise chain ends with a catch so the ignored finally promise
cannot reject unhandled. Apply the same recovery to the corresponding wake path
noted at the additional location.
- Around line 3118-3120: Update the issue-key parser/normalizer feeding
durableIssue and babysitterCritical to accept GitHub keys in
owner/repository#number format, such as AgentWorkforce/hoopsheet#10, alongside
existing AR and numeric keys. Preserve normalization and ensure the resulting
key is recognized by both destructive-operation fence checks, including the
corresponding logic near the alternate location.

In `@src/state/file-state-store.ts`:
- Line 566: Update the persisted-record validation around candidate.critical so
missing or undefined values are rejected, requiring critical to be a boolean
rather than accepting undefined and restoring false. Apply the same strict
validation to the corresponding check at the other referenced location, while
preserving existing handling for valid boolean values.

In `@src/triage/schema.ts`:
- Around line 18-27: Remove ownedPullRequest and pendingPullRequestWake from
TriageDecisionSchema, keeping them available only on the internal AgentSpec
shape. Update LlmTriage or the decision-to-agent-state storage path to strip or
otherwise prevent these fields from parsed model output, while preserving
babysitter lifecycle code as the only source that sets them.

---

Outside diff comments:
In `@src/orchestrator/factory.ts`:
- Around line 7676-7679: Update parsePullSnapshot so fallbackNumber from the
canonical path is authoritative, rather than allowing payload.number to override
it. Validate the payload number when present and return undefined if it is not
an integer or disagrees with fallbackNumber; otherwise continue using
fallbackNumber for the resulting PullSnapshot.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 726c270d-7a63-421f-9dde-2fbe7b8f45fc

📥 Commits

Reviewing files that changed from the base of the PR and between 0585332 and e41931e.

📒 Files selected for processing (10)
  • src/dispatch/templates.test.ts
  • src/dispatch/templates.ts
  • src/orchestrator/factory.test.ts
  • src/orchestrator/factory.ts
  • src/ports/fleet.ts
  • src/ports/state.ts
  • src/state/file-state-store.test.ts
  • src/state/file-state-store.ts
  • src/state/in-memory-state-store.ts
  • src/triage/schema.ts

Comment thread src/orchestrator/factory.ts
Comment thread src/orchestrator/factory.ts
Comment thread src/orchestrator/factory.ts
Comment thread src/orchestrator/factory.ts
Comment thread src/orchestrator/factory.ts
Comment thread src/orchestrator/factory.ts
Comment thread src/state/file-state-store.ts Outdated
Comment thread src/triage/schema.ts Outdated
@miyaontherelay

Copy link
Copy Markdown
Contributor Author

Addressed the outside-diff PR-number finding in 7bb575d: the canonical path number is authoritative; an explicit payload number must parse as a positive integer and equal the path, otherwise the snapshot is rejected. Added a path/payload mismatch regression. All nine CodeRabbit findings were independently audited and signed off on exact freeze 7bb575d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Babysitter goes blind after spawn: route its own PR's review/CI events into its session instead of dropping them

1 participant